Skip to content

fix!: Fix LicenseStatus response and Supportkey type#4297

Merged
gmlewis merged 8 commits into
google:masterfrom
s4mur4i:master
Jun 16, 2026
Merged

fix!: Fix LicenseStatus response and Supportkey type#4297
gmlewis merged 8 commits into
google:masterfrom
s4mur4i:master

Conversation

@s4mur4i

@s4mur4i s4mur4i commented Jun 12, 2026 •
edited by gmlewis
Loading

Copy link
Copy Markdown
Contributor

BREAKING CHANGE: LicenseStatus.SupportKey type changed from *string to *bool and License return type is no longer a slice.

Closes: #4296

License status response was implemented according to documentation, which is not according to actual response.

Fix:
Corrected the response
Fixed incorrect data type for SupportKey
Fixed tests

@gmlewis gmlewis changed the title fix: Fix License Status response and Supportkey type fix!: Fix License Status response and Supportkey type Jun 12, 2026
@gmlewis gmlewis changed the title fix!: Fix License Status response and Supportkey type fix!: Fix LicenseStatus response and Supportkey type Jun 12, 2026
@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Jun 12, 2026
@codecov

codecov Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.47%. Comparing base (952dff0) to head (aefc472).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #4297   +/-   ##
=======================================
  Coverage   97.46%   97.47%           
=======================================
  Files         192      192           
  Lines       19258    19310   +52     
=======================================
+ Hits        18770    18822   +52     
  Misses        270      270           
  Partials      218      218

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gmlewis
gmlewis approved these changes Jun 12, 2026
View reviewed changes

@gmlewis gmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @s4mur4i!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra

alexandear
alexandear reviewed Jun 12, 2026
View reviewed changes
Comment thread github/enterprise_manage_ghes_config.go
Comment thread github/enterprise_manage_ghes_config.go Outdated
alexandear
alexandear reviewed Jun 15, 2026
View reviewed changes
Comment thread github/enterprise_manage_ghes_config.go Outdated
Comment thread github/enterprise_manage_ghes_config.go Outdated
s4mur4i and others added 2 commits June 15, 2026 18:02
@s4mur4i @alexandear
Update github/enterprise_manage_ghes_config.go
58a9e83 
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
@s4mur4i
fix: Update note
6bfe99b
alexandear
alexandear reviewed Jun 16, 2026
View reviewed changes
Comment thread github/enterprise_manage_ghes_config.go Outdated
Comment on lines +73 to +74
// SupportKey is documented as string but is actual a bool.
// TODO: Remove comment after github updates schema documentation

@alexandear alexandear Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer needed.

@gmlewis gmlewis Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s4mur4i - please remove these two lines.

@s4mur4i s4mur4i Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline version was failing lint, so I moved it to header par, similar to licensestatus.
As I see lint output that part doesnt fail.

@alexandear alexandear Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have plenty of inline comments for fields - they all passed linting. E.g.,

go-github/github/billing.go

Lines 131 to 132 in 0eacd34

// Organization name is only used for organization-level reports.
OrganizationName *string `json:"organizationName,omitempty"`

@s4mur4i s4mur4i Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it back, Can linter be run again please.

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Jun 16, 2026
@s4mur4i

s4mur4i commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@gmlewis I added a commit for fixing the lint findings. Hope it will be good like this.

@alexandear

alexandear commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

The linter is failing because of formatting issues. You need to run ./script/fmt.sh

@s4mur4i

s4mur4i commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

The linter is failing because of formatting issues. You need to run ./script/fmt.sh

Ran it, commited results.

@gmlewis

gmlewis commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Thank you, @s4mur4i and @alexandear!
Merging.

@gmlewis gmlewis merged commit 98eb342 into google:master Jun 16, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmlewis gmlewis gmlewis approved these changes

+1 more reviewer

@alexandear alexandear alexandear approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect return and data type in license api

3 participants

@s4mur4i @alexandear @gmlewis